Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

llvmPackages_14: Fix build on aarch64-linux #201485

Merged
merged 2 commits into from
Nov 30, 2022

Conversation

avdv
Copy link
Member

@avdv avdv commented Nov 16, 2022

Description of changes

This PR fixes building the LLVM packages on aarch64-linux.

The libcxx library needs a C++20 capable compiler toolchain since version 14.

Also, work around the linking problems described in #201254

ZHF: #199919

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@risicle
Copy link
Contributor

risicle commented Nov 16, 2022

@ofborg eval

@risicle
Copy link
Contributor

risicle commented Nov 17, 2022

Hmm I'm still getting

/nix/store/1g5g7vzlcprmg1d6864fs5r4nf1h5b5j-binutils-2.39/bin/ld: CMakeFiles/clang_rt.scudo_standalone-dynamic-aarch64.dir/linux.cpp.o: in function `bool scudo::atomic_compare_exchange_strong<scudo::atomic_u32>(scudo::atomic_u32 volatile*, scudo::atomic_u32::Type*, scudo::atomic_u32::Type, scudo::memory_order)':
/build/compiler-rt-src-14.0.6/compiler-rt/lib/scudo/standalone/atomic_helpers.h:119: undefined reference to `__aarch64_cas4_acq'

for compiler-rt-libc-14.0.6

@avdv
Copy link
Member Author

avdv commented Nov 18, 2022

@risicle you're right, I have added the same workaround for compiler-rt and clang.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild and removed 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ labels Nov 18, 2022
@avdv
Copy link
Member Author

avdv commented Nov 19, 2022

When building llvmPackages_14.libcxxabi, I ran into this error:

n file included from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/__compare/strong_order.h:12,
                 from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/__compare/weak_order.h:14,
                 from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/__compare/partial_order.h:14,
                 from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/__compare/compare_partial_order_fallback.h:13,                   
                 from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/compare:144,                                         
                 from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/utility:236,
                 from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/__functional_base:26,
                 from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/memory:808,         
                 from /build/libcxxabi-src-14.0.6/libcxxabi/../libcxx/src/include/atomic_support.h:13,
                 from /build/libcxxabi-src-14.0.6/libcxxabi/src/cxa_default_handlers.cpp:19:
/nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/__bit/bit_cast.h: In function 'constexpr _ToType std::__1::bit_cast(const _FromType&)':
/nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/__bit/bit_cast.h:31:38: error: expected primary-expression before ',' token
   31 |     return __builtin_bit_cast(_ToType, __from);
      |                                      ^                                                                                                                                   
In file included from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/__compare/strong_order.h:12,
                 from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/__compare/weak_order.h:14,
                 from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/__compare/partial_order.h:14,                                             
                 from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/__compare/compare_partial_order_fallback.h:13,                
                 from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/compare:144,
                 from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/utility:236,
                 from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/__functional_base:26,        
                 from /nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/memory:808,
                 from /build/libcxxabi-src-14.0.6/libcxxabi/../libcxx/src/include/atomic_support.h:13,
                 from /build/libcxxabi-src-14.0.6/libcxxabi/src/cxa_default_handlers.cpp:19:         
/nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/__bit/bit_cast.h: In function 'constexpr _ToType std::__1::bit_cast(const _FromType&)':
/nix/store/87sa00y1dyyfyigqzav5cjjgzrr44rx4-libcxx-headers-14.0.6/include/c++/v1/__bit/bit_cast.h:31:38: error: expected primary-expression before ',' token
   31 |     return __builtin_bit_cast(_ToType, __from);                                                                                                                          
      |                                      ^                                                                                                                                   
make[2]: *** [src/CMakeFiles/cxxabi_static.dir/build.make:90: src/CMakeFiles/cxxabi_static.dir/cxa_default_handlers.cpp.o] Error 1               
make[1]: *** [CMakeFiles/Makefile2:165: src/CMakeFiles/cxxabi_static.dir/all] Error 2                                                                                            
make[1]: *** Waiting for unfinished jobs....                                                                                                                                     
make[2]: *** [src/CMakeFiles/cxxabi_shared.dir/build.make:90: src/CMakeFiles/cxxabi_shared.dir/cxa_default_handlers.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....                                                                    

According to llvm/llvm-project#56828 this happens with GCC 10, but works OK with GCC 11.

@risicle
Copy link
Contributor

risicle commented Nov 19, 2022

Linking error again for lldb

@@ -26,6 +26,9 @@ let

buildInputs = [ libxml2 libllvm ];

# workaround https://github.com/NixOS/nixpkgs/issues/201254
NIX_LDFLAGS = if stdenv.isLinux && stdenv.isAarch64 && stdenv.cc.isGNU then "-lgcc" else null;
Copy link
Contributor

@ehmry ehmry Nov 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should consider if adding an argument above like { withLibgcc ? stdenv.isLinux && stdenv.isAarch64 && stdenv.cc.isGNU, ... }: is better than putting these conditions inline in the body of derivations. This could make overriding LLVM easier and I think its self-documenting to have code in the form of effect ? cause rather than if ${cause} then ${action} else null.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is just an ugly workaround, I would prefer to keep this as ugly as possible. 😬

@avdv
Copy link
Member Author

avdv commented Nov 21, 2022

Linking error again for lldb

Fixed.

I tried to use nixpkgs-review, but this never finishes always crashes with a OOM error... 🤷

Copy link
Member

@primeos primeos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as a TODO: The commit history needs to be cleaned up (one commit per logical change - currently there are basically a few fixup commits - I'd suggest to either put everything into one commit or one to fix the build and one for the -lgcc changes (the latter would probably be better)).

Ideally this should also target llvmPackages_git as well (normally that would be required but llvmPackages_git is in bad shape since many PRs forget to target it and llvmPackages_15 already has a PR that is stuck in limbo...).

pkgs/development/compilers/llvm/14/clang/default.nix Outdated Show resolved Hide resolved
@rrbutani rrbutani mentioned this pull request Nov 27, 2022
92 tasks
The libcxx library needs a C++20 capable compiler toolchain since version 14, but aarch64-linux uses GCC 9 by default.

However, building libcxxabi fails when using GCC 10, see [llvm/llvm-project#56828]. So let's use GCC 11 instead.

[llvm/llvm-project#56828]: llvm/llvm-project#56828
@avdv
Copy link
Member Author

avdv commented Nov 28, 2022

@primeos I have rebased onto master and refined the commit history.

@ofborg ofborg bot requested a review from primeos November 28, 2022 08:24
Copy link
Member

@primeos primeos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now, thanks! :)

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/694

@vcunat
Copy link
Member

vcunat commented Nov 30, 2022

This can't go directly to master. OfBorg doesn't check this for aarch64-linux, but it is a large rebuild:

Estimating rebuild amount by counting changed Hydra jobs (parallel=unset).
  14523 aarch64-linux

@vcunat vcunat changed the base branch from master to staging November 30, 2022 10:14
@ofborg ofborg bot requested a review from primeos November 30, 2022 10:29
@ehmry ehmry merged commit 38c793c into NixOS:staging Nov 30, 2022
@avdv avdv deleted the fix-llvm_14-aarch64-linux branch December 1, 2022 09:23
@mweinelt
Copy link
Member

Bisected a broken llvm_14 on aarch64-linux to 7496574.

In https://hydra.nixos.org/build/201827173 it fails >80 tests.

@ehmry
Copy link
Contributor

ehmry commented Dec 12, 2022

Go ahead and revert my merge if it doesn't make things worse.

Comment on lines +14752 to +14753
} // lib.optionalAttrs (stdenv.hostPlatform.isAarch64 && stdenv.hostPlatform.isLinux && buildPackages.stdenv.cc.isGNU) {
stdenv = gcc11Stdenv;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avdv
Copy link
Member Author

avdv commented Dec 13, 2022

Bisected a broken llvm_14 on aarch64-linux to 7496574.

In hydra.nixos.org/build/201827173 it fails >80 tests.

It fails all the gold tests. Which is caused by:

/nix/store/cwivsnh1p591x7yydynrz0xc1zcq54p3-binutils-2.39/bin/ld.gold: error: /build/llvm-src-14.0.6/llvm/build/lib/LLVMgold.so: could not load plugin library: /nix/store/kyrb20x8ldfhih953ffd8yba42qfb5xy-gcc-9.5.0-lib/lib/libstdc++.so.6: version `GLIBCXX_3.4.29' not found (required by /build/llvm-src-14.0.6/llvm/build/lib/libLLVM-14.so)

Would it be feasible to disable the gold linker plugin on this platform?

vcunat added a commit that referenced this pull request Dec 13, 2022
This reverts commits 7496574 and 38c793c.
llvm_14 wouldn't even build on aarch64-linux (test phase).
@vcunat
Copy link
Member

vcunat commented Dec 13, 2022

I don't really understand this, so I just reverted the whole PR for now. Otherwise llvm_14 doesn't build on aarch64-linux – and thus (almost?) all llvmPackages_14, so this PR seems to lose its point.

winterqt added a commit to winterqt/nixpkgs that referenced this pull request Jan 4, 2023
This change switches to using GCC 11 by default on aarch64-linux, as well as passing `-lgcc` to the linker, per NixOS#201485.

See NixOS#201254 and NixOS#208412 for wider context on the issue.
winterqt added a commit to winterqt/nixpkgs that referenced this pull request Feb 15, 2023
This change switches to using GCC 11 by default on aarch64-linux, as well as passing `-lgcc` to the linker, per NixOS#201485.

See NixOS#201254 and NixOS#208412 for wider context on the issue.

(cherry picked from commit 8442601)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants